Authenticate Requests from ApiServerSources by Leo6Leo · Pull Request #7452 · knative/eventing

Leo6Leo · 2023-11-15T13:20:51Z

Proposed Changes

Prerequisite: When OIDC mode is enabled and sink has audience

All the outgoing event request will be appended with JWT Authorization header
Automatically create role and rolebinding for the source's serviceaccount so that they have permission to create the JWT token for it's OIDC service account and the audience
Added the tests for role and rolebinding creation

Pre-review Checklist

At least 80% unit test coverage
E2E tests for any new behavior
Docs PR for any user-facing impact
Spec PR for any new API feature
Conformance test for any change to the spec

Release Note

Under OIDC mode, all the outgoing event request will be appended with JWT Authorization header

Docs

knative-prow · 2023-11-15T13:20:54Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

codecov · 2023-11-15T13:27:16Z

Codecov Report

Attention: 143 lines in your changes are missing coverage. Please review.

Comparison is base (5d7e104) 76.19% compared to head (a73a26b) 75.53%.
Report is 1 commits behind head on main.

❗ Current head a73a26b differs from pull request most recent head 8890c5d. Consider uploading reports for the commit 8890c5d to get more accurate results

Files	Patch %	Lines
...iler/apiserversource/resources/oidc_rolebinding.go	0.00%	69 Missing ⚠️
pkg/reconciler/apiserversource/apiserversource.go	55.12%	27 Missing and 8 partials ⚠️
pkg/adapter/v2/cloudevents.go	38.23%	19 Missing and 2 partials ⚠️
...ciler/apiserversource/resources/receive_adapter.go	14.28%	10 Missing and 2 partials ⚠️
pkg/adapter/v2/config.go	40.00%	5 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7452      +/-   ##
==========================================
- Coverage   76.19%   75.53%   -0.66%     
==========================================
  Files         260      261       +1     
  Lines       14447    14667     +220     
==========================================
+ Hits        11008    11079      +71     
- Misses       2873     3008     +135     
- Partials      566      580      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/adapter/apiserver/delegate.go

Signed-off-by: Leo Li <leoli@redhat.com>

SECURITY.md

pkg/adapter/v2/cloudevents.go

pkg/auth/serviceaccount.go

pkg/reconciler/inmemorychannel/controller/resources/role_binding.go

pkg/adapter/apiserver/delegate.go

pkg/adapter/v2/cloudevents.go

Signed-off-by: Leo Li <leoli@redhat.com>

Co-authored-by: Christoph Stäbler <cstabler@redhat.com>

Co-authored-by: Calum Murray <cmurray@redhat.com>

Signed-off-by: Leo Li <leoli@redhat.com>

…r for the role and rolebinding informers Signed-off-by: Leo Li <leoli@redhat.com>

creydr

Left some comments on the role & rolebinding event handler filters.

You also need to adjust the tests too (added labels and later with the new owner reference)

pkg/reconciler/apiserversource/controller.go

Signed-off-by: Leo Li <leoli@redhat.com>

cmd/controller/main.go

pkg/reconciler/apiserversource/apiserversource_test.go

pkg/reconciler/apiserversource/controller_test.go

Signed-off-by: Leo Li <leoli@redhat.com>

pkg/apis/sources/register.go

…using only the selector oidc) and not by value Signed-off-by: Leo Li <leoli@redhat.com>

Signed-off-by: Leo Li <leoli@redhat.com>

creydr

Thanks a lot @Leo6Leo for your hard work and patience on all the review cycles on this!

/lgtm

🎉🎉🎉

knative-prow · 2023-12-18T17:07:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: creydr, Leo6Leo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [creydr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Add the task planning toughts to the PR

e2092b0

knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 15, 2023

knative-prow bot requested review from Cali0707 and odacremolbap November 15, 2023 13:20

knative-prow bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 15, 2023

creydr reviewed Nov 15, 2023

View reviewed changes

pkg/adapter/apiserver/delegate.go Outdated Show resolved Hide resolved

Leo6Leo added 2 commits November 15, 2023 14:13

Merge branch 'main' into authenticate-request-from-apiserversource

2369d50

add the auth header to all the send and request to the adapter

48c349f

knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 16, 2023

Leo6Leo added 3 commits November 17, 2023 16:06

Merge branch 'main' into authenticate-request-from-apiserversource

0e83dbc

Adding the env var for K_AUDIENCE

e7b38a8

Adding the rekt test

8efcee8

knative-prow bot added area/test-and-release Test infrastructure, tests or release size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 20, 2023

Leo6Leo added 4 commits November 23, 2023 14:19

Merge the upstream main branch

aea806a

Adding the rolebinding for create_oidc

e11c5e9

Signed-off-by: Leo Li <leoli@redhat.com>

Adding the rolebinding for create_oidc

8b4463c

Signed-off-by: Leo Li <leoli@redhat.com>

Update the env variable

ef855c8

Signed-off-by: Leo Li <leoli@redhat.com>

creydr reviewed Nov 24, 2023

View reviewed changes

SECURITY.md Show resolved Hide resolved

pkg/adapter/v2/cloudevents.go Outdated Show resolved Hide resolved

pkg/auth/serviceaccount.go Outdated Show resolved Hide resolved

pkg/reconciler/inmemorychannel/controller/resources/role_binding.go Show resolved Hide resolved

Cali0707 reviewed Nov 24, 2023

View reviewed changes

pkg/adapter/apiserver/delegate.go Show resolved Hide resolved

Cali0707 reviewed Nov 24, 2023

View reviewed changes

pkg/adapter/v2/cloudevents.go Outdated Show resolved Hide resolved

Leo6Leo and others added 4 commits November 24, 2023 11:19

Revert the automated change to the file

5c134b3

Signed-off-by: Leo Li <leoli@redhat.com>

Update pkg/adapter/apiserver/delegate.go

4d3caec

Co-authored-by: Christoph Stäbler <cstabler@redhat.com>

Apply suggestions from code review

0cab4e5

Co-authored-by: Calum Murray <cmurray@redhat.com>

Merge the upstream main branch

c6a9d54

Signed-off-by: Leo Li <leoli@redhat.com>

knative-prow bot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 30, 2023

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2023

Change to use the filtered informer, and add the informer eventhandle…

479aa7f

…r for the role and rolebinding informers Signed-off-by: Leo Li <leoli@redhat.com>

Leo6Leo force-pushed the authenticate-request-from-apiserversource branch from bb7c0d6 to 479aa7f Compare December 13, 2023 21:42

Change to use the filtered informer, and add the informer eventhandle…

35c43b6

…r for the role and rolebinding informers Signed-off-by: Leo Li <leoli@redhat.com>

creydr requested changes Dec 14, 2023

View reviewed changes

pkg/reconciler/apiserversource/controller.go Show resolved Hide resolved

pkg/reconciler/apiserversource/controller.go Show resolved Hide resolved

Leo6Leo added 4 commits December 14, 2023 10:33

Merge branch 'main' into authenticate-request-from-apiserversource

757880e

Inject the oidcSelector in the main.go

c8254e5

Signed-off-by: Leo Li <leoli@redhat.com>

Add the owner reference to the role and rolebinding

30b7d22

Signed-off-by: Leo Li <leoli@redhat.com>

Update the tests

72b37ed

Signed-off-by: Leo Li <leoli@redhat.com>

knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 14, 2023

Update the tests

55c1724

Signed-off-by: Leo Li <leoli@redhat.com>

Leo6Leo requested a review from creydr December 15, 2023 06:43

creydr reviewed Dec 15, 2023

View reviewed changes

Leo6Leo added 3 commits December 15, 2023 10:41

nit fix

0d053b3

Signed-off-by: Leo Li <leoli@redhat.com>

register the selectors

c082ede

Signed-off-by: Leo Li <leoli@redhat.com>

nit: fix the go imports

62fe570

Signed-off-by: Leo Li <leoli@redhat.com>

Leo6Leo requested a review from creydr December 18, 2023 06:03

creydr reviewed Dec 18, 2023

View reviewed changes

pkg/apis/sources/register.go Outdated Show resolved Hide resolved

Leo6Leo added 4 commits December 18, 2023 08:48

Merge branch 'main' into authenticate-request-from-apiserversource

a8de8b8

nit: renaming the variables and only filter on the labelKey (e.g. by …

f8b4795

…using only the selector oidc) and not by value Signed-off-by: Leo Li <leoli@redhat.com>

nit: renaming the variables and only filter on the labelKey (e.g. by …

a73a26b

…using only the selector oidc) and not by value Signed-off-by: Leo Li <leoli@redhat.com>

nit: fix the naming and add - to the create-oidc-token

8890c5d

Signed-off-by: Leo Li <leoli@redhat.com>

creydr approved these changes Dec 18, 2023

View reviewed changes

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Dec 18, 2023

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 18, 2023

knative-prow bot merged commit 31abcdb into knative:main Dec 18, 2023

This was referenced Dec 20, 2023

Use filtered informer to watch OIDC service accounts #7341

Closed

Authenticate Requests from PingSources #7320

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Authenticate Requests from ApiServerSources#7452

Authenticate Requests from ApiServerSources#7452
knative-prow[bot] merged 73 commits intoknative:mainfrom
Leo6Leo:authenticate-request-from-apiserversource

Leo6Leo commented Nov 15, 2023 •

edited

Loading

Uh oh!

knative-prow bot commented Nov 15, 2023

Uh oh!

codecov bot commented Nov 15, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

creydr left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

creydr left a comment

Uh oh!

knative-prow bot commented Dec 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Leo6Leo commented Nov 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Pre-review Checklist

Uh oh!

knative-prow bot commented Nov 15, 2023

Uh oh!

codecov bot commented Nov 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

creydr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

creydr left a comment

Choose a reason for hiding this comment

Uh oh!

knative-prow bot commented Dec 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Leo6Leo commented Nov 15, 2023 •

edited

Loading

codecov bot commented Nov 15, 2023 •

edited

Loading